Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

B 20653 mobile home shipment card #13620

Merged
merged 15 commits into from
Sep 10, 2024
Merged

B 20653 mobile home shipment card #13620

merged 15 commits into from
Sep 10, 2024

Conversation

KonstanceH
Copy link
Contributor

@KonstanceH KonstanceH commented Sep 3, 2024 •
edited
Loading

B-20653

Summary

This PR updates the mobile home shipment status to 'SUBMITTED' when the shipment is submitted by the customer.
This also removes 'Please' from the shipment option description, and updates the shipment opiton description to match the PO AC.
Any labeling for Mobile home that's user facing is now "Mobile Home".

Verification Steps for Reviewers

These are to be checked by a reviewer.

  • Has the branch been pulled in and checked out?
  • Have the BL acceptance criteria been met for this change?
  • Was the CircleCI build successful?
  • Has the code been reviewed from a standards and best practices point of view?

How to test

  1. Login as a customer
  2. Add a mobile home shipment
  3. Confirm the labeling is "Mobile Home"
  4. Confirm description changes described above
  5. Complete the move and sign
  6. Confirm in DB that the mtoshipment status for that mobile home shipment is set to 'SUBMITTED'

Frontend

  • There are no aXe warnings for UI.
  • This works in Supported Browsers and their phone views (Chrome, Firefox, Edge).
  • There are no new console errors in the browser devtools.
  • There are no new console errors in the test output.
  • If this PR adds a new component to Storybook, it ensures the component is fully responsive, OR if it is intentionally not, a wrapping div using the officeApp class or custom min-width styling is used to hide any states the would not be visible to the user.
  • This change meets the standards for Section 508 compliance.

Backend

ANDI Screenshots

Removed location Page title
Screenshot 2024-09-05 at 10 13 10 AM

Removed Please from shipment list description
Screenshot 2024-09-05 at 8 55 38 AM

Change of dimension description
Screenshot 2024-09-03 at 3 36 21 PM

Shipment Option label change
Screenshot 2024-09-03 at 3 35 26 PM

Shipment Card label change
Screenshot 2024-09-03 at 3 34 36 PM

@KonstanceH KonstanceH self-assigned this Sep 3, 2024
@KonstanceH KonstanceH added ByteSize M&Ms Team ByteSized M&Ms INTEGRATION Slated for Integration Testing labels Sep 3, 2024
@robot-mymove
Copy link

robot-mymove commented Sep 3, 2024 •
edited
Loading

Warnings
⚠️ This PR does not include changes to storybook, even though it affects component code.

Generated by 🚫 dangerJS against 2258a2f

@KonstanceH KonstanceH marked this pull request as ready for review September 5, 2024 13:44
@KonstanceH KonstanceH requested a review from a team as a code owner September 5, 2024 13:44
@taeJungCaci
Copy link
Contributor

taeJungCaci commented Sep 5, 2024 •
edited
Loading

Some of the things I've noticed while testing the functionality - There is a remarks field on the 1st page and another on the 2nd page which can be removed. And also when I edit, the pickup address doesn't seem to populate with existing data.

@KonstanceH
Copy link
Contributor Author

KonstanceH commented Sep 5, 2024 •
edited
Loading

Some of the things I've noticed while testing the functionality - There is a remarks field on the 1st page and another on the 2nd page which can be removed. And also when I edit, the pickup address doesn't seem to populate with existing data.

Ah actually I see why they gotta go I have the remarks already on the location page, I'll get rid of those.
I feel like I should change the description on them because I left the formatting the same as the boat shipment though lol

I'm working on seeing why the pickupaddress isn't populating on edit now

@taeJungCaci
Copy link
Contributor

taeJungCaci commented Sep 6, 2024 •
edited
Loading

Some of the things I've noticed while testing the functionality - There is a remarks field on the 1st page and another on the 2nd page which can be removed. And also when I edit, the pickup address doesn't seem to populate with existing data.

Ah actually I see why they gotta go I have the remarks already on the location page, I'll get rid of those. I feel like I should change the description on them because I left the formatting the same as the boat shipment though lol

I'm working on seeing why the pickupaddress isn't populating on edit now

Yeah, sorry I could have been more clear. For boat I have the remarks on the 1st page and not on the location page. And for the description - yeah... not sure because the boat story had a mockup with those description already in it, but I don't see one for Mobile home.

@taeJungCaci taeJungCaci requested review from JamesHawks224 and removed request for JamesHawks224 September 6, 2024 13:44
@JamesHawks224
Copy link
Contributor

image
"home" in "Mobile home Information" should be capitalized.

JamesHawks224
JamesHawks224 requested changes Sep 6, 2024
View reviewed changes
Copy link
Contributor

@JamesHawks224 JamesHawks224 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing: Success w/1 issue (see comment)
Code: lgtm

traskowskycaci
traskowskycaci reviewed Sep 6, 2024
View reviewed changes
Copy link
Contributor

@traskowskycaci traskowskycaci left a comment •
edited by WeatherfordAaron
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionality looks good! Code looks good and meets AC. I did notice the AC is
"Given a Mobile Home shipment has been created, 
When a customer reviews their boat shipment,"
and that When should probably refer to the mobile home shipment rather than boat

Inserting myself into this PR so forgive me if any of these are supposed to be this way, but noticed a few other small things. If all instances of the words "mobile home" should be "Mobile Home" I see a few spots without that capitalization - "Mobile home Information", "Mobile home details and measurements"
image

Also not sure if they need to be capitalized mid-sentence down in the remarks section here. However, I do see a typo where "signigicantly" should be "significantly"
image

@traskowskycaci
Copy link
Contributor

Also not sure if they need to be capitalized mid-sentence down in the remarks section here. However, I do see a typo where "signigicantly" should be "significantly" image

If you think that significantly typo is out of scope for this PR, let me know and I can get an issue created. Looks like it would be a typo for both mobile and boat shipment remarks

traskowskycaci
traskowskycaci requested changes Sep 6, 2024
View reviewed changes
Copy link
Contributor

@traskowskycaci traskowskycaci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more comment because I am also looking at Aaron's changes for the office users - can we make the destination address field match? I'm good with either Destination or Destination address, whatever you both agree on. Just would like it to be the same if it's not too big of a deal :)

Destination for customer on the left bottom vs Destination address for office user on the right bottom
image

cc @WeatherfordAaron

@traskowskycaci
Copy link
Contributor

B-20653

D

One more comment because I am also looking at Aaron's changes for the office users - can we make the destination address field match? I'm good with either Destination or Destination address, whatever you both agree on. Just would like it to be the same if it's not too big of a deal :)

Destination for customer on the left bottom vs Destination address for office user on the right bottom image

cc @WeatherfordAaron

Disregard, you're meeting both meeting AC with what you have

@KonstanceH
Copy link
Contributor Author

image "home" in "Mobile home Information" should be capitalized.

ah I see I'm on it

@KonstanceH
Copy link
Contributor Author

Functionality looks good! Code looks good and meets AC. I did notice the AC is "Given a Mobile Home shipment has been created,  When a customer reviews their boat shipment," and that When should probably refer to the mobile home shipment rather than boat

Inserting myself into this PR so forgive me if any of these are supposed to be this way, but noticed a few other small things. If all instances of the words "mobile home" should be "Mobile Home" I see a few spots without that capitalization - "Mobile home Information", "Mobile home details and measurements" image

Also not sure if they need to be capitalized mid-sentence down in the remarks section here. However, I do see a typo where "signigicantly" should be "significantly" image

Yea that's definitely fair. I'm gonna change them all to follow "Mobile Home"

@WeatherfordAaron
Copy link
Contributor

Functionality looks good! Code looks good and meets AC. I did notice the AC is "Given a Mobile Home shipment has been created,  When a customer reviews their boat shipment," and that When should probably refer to the mobile home shipment rather than boat
Inserting myself into this PR so forgive me if any of these are supposed to be this way, but noticed a few other small things. If all instances of the words "mobile home" should be "Mobile Home" I see a few spots without that capitalization - "Mobile home Information", "Mobile home details and measurements" image
Also not sure if they need to be capitalized mid-sentence down in the remarks section here. However, I do see a typo where "signigicantly" should be "significantly" image

Yea that's definitely fair. I'm gonna change them all to follow "Mobile Home"

If you haven't already, updating this branch might fix some of it.

@WeatherfordAaron
Copy link
Contributor

Functionality looks good! Code looks good and meets AC. I did notice the AC is "Given a Mobile Home shipment has been created,  When a customer reviews their boat shipment," and that When should probably refer to the mobile home shipment rather than boat

Inserting myself into this PR so forgive me if any of these are supposed to be this way, but noticed a few other small things. If all instances of the words "mobile home" should be "Mobile Home" I see a few spots without that capitalization - "Mobile home Information", "Mobile home details and measurements" image

Also not sure if they need to be capitalized mid-sentence down in the remarks section here. However, I do see a typo where "signigicantly" should be "significantly" image

I agree.. it should be Mobile Home Information instead of Mobile home Information.
Anytime "mobile" and "home" are together in reference to the type of shipment, they should both be capitalized. The only caveat is when we are referring to the customer's mobile home.

WeatherfordAaron
WeatherfordAaron approved these changes Sep 9, 2024
View reviewed changes
Copy link
Contributor

@WeatherfordAaron WeatherfordAaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm once you get any straggling errors cleaned up 👽

traskowskycaci
traskowskycaci approved these changes Sep 9, 2024
View reviewed changes
Copy link
Contributor

@traskowskycaci traskowskycaci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

@JamesHawks224
Copy link
Contributor

Testing: Success w/1 issue (see comment) Code: lgtm

Checked update. issue resolved.

@KonstanceH KonstanceH merged commit f8fe598 into integrationTesting Sep 10, 2024
30 checks passed
@KonstanceH KonstanceH deleted the B-20653-Mobile-Home-Shipment-Card branch September 10, 2024 17:10
@KonstanceH KonstanceH mentioned this pull request Sep 11, 2024
Merged
9 tasks
@KonstanceH KonstanceH mentioned this pull request Sep 25, 2024
Closed
This was referenced Oct 4, 2024
Closed
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JamesHawks224 JamesHawks224 JamesHawks224 approved these changes

@traskowskycaci traskowskycaci traskowskycaci approved these changes

@WeatherfordAaron WeatherfordAaron WeatherfordAaron approved these changes

@taeJungCaci taeJungCaci Awaiting requested review from taeJungCaci

Assignees

@KonstanceH KonstanceH

Labels
ByteSize M&Ms Team ByteSized M&Ms INTEGRATION Slated for Integration Testing
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@KonstanceH @robot-mymove @taeJungCaci @JamesHawks224 @traskowskycaci @WeatherfordAaron